feat: observe resize segment for better indicator display #565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dskloetd
reviewed
Jan 21, 2025
dskloetd
approved these changes
Jan 21, 2025
1 task
github-merge-queue bot
pushed a commit
to dfinity/nns-dapp
that referenced
this pull request
Feb 4, 2025
# Motivation Currently, there are large side gaps on mobile that take up too much space. This PR improves the UI by reducing those gaps, allowing more content to be displayed on mobile. Demo: https://qsgjb-riaaa-aaaaa-aaaga-cai.mstr-ingress.devenv.dfinity.network/ # Changes - npm run upgrade:gix to fetch [related changes](dfinity/gix-components#572). - Update mobile styles: - Add a margin to the filter buttons because they have a negative margin. Otherwise, their borders are partly hidden. - Align the universe selector with the content width. - Avoid extra padding on portfolio page. # Tests - ~Add global ResizeObserver mock, because of [changes in gix-components](dfinity/gix-components#565 Not needed [anymore](#6320). - Still pass. - Screenshots updated. - Tested manually. | Before | After | |--------|--------| | <img width="385" alt="image" src="https://github.com/user-attachments/assets/5215a783-c567-4374-be93-98505ab22b86" /> | <img width="388" alt="image" src="https://github.com/user-attachments/assets/1580f18e-9158-49c6-be99-4782c530cbf1" /> | | <img width="387" alt="image" src="https://github.com/user-attachments/assets/96eb9131-48c9-4953-954c-05dce5ffd73d" /> | <img width="386" alt="image" src="https://github.com/user-attachments/assets/2ecb5e16-5553-4a8b-8949-dce90f106f93" /> | | <img width="385" alt="image" src="https://github.com/user-attachments/assets/14ce43ac-a5e7-47af-afe8-16a26e146f9d" /> | <img width="387" alt="image" src="https://github.com/user-attachments/assets/458d3157-9f70-4327-8ee7-f3fc60b5b860" /> | | <img width="388" alt="image" src="https://github.com/user-attachments/assets/bd1b6eda-e9cb-4cbc-87b2-66f95ce531b6" /> | <img width="386" alt="image" src="https://github.com/user-attachments/assets/2a9c44cc-d2d9-481d-b699-68cc15bc9c3b" /> | # Todos - [ ] Add entry to changelog (if necessary). not necessary
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Tests in #563 were failing. After debugging, I figured out that the potential display issue actually already existed on the main branch. Specifically, since the segment buttons have no fixed width, depending on the rendering, it's possible that on mount, the buttons are not yet fully rendered—meaning they haven't occupied their full width yet—resulting in the indicator position not being accurately calculated. Similarly, the buttons might be rendered, but the font could take time to load, causing the indicator position to be calculated with the fallback font instead of the final one. Additionally, if the language is changed at runtime, it might result in a change in the buttons' length.
Long story short, observing the size of the container and repositioning the indicator when the size changes ensures that the indicator is always correctly positioned.
Changes
ResizeObserverto set the indicator position.ResizeObserverglobally for vitestScreenshots
Before on main:
After:
Tests
Tests of #563 should pass.